Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

New feature: added partial postcode match to table rate shipping #1504

Merged
merged 5 commits into from
Feb 21, 2024
Merged

New feature: added partial postcode match to table rate shipping #1504

merged 5 commits into from
Feb 21, 2024

Conversation

AlterWeb
Copy link
Contributor

@AlterWeb AlterWeb commented Mar 18, 2021 •
edited
Loading

Description (*)

Added partial postcode match to table rate shipping.

Needed to create shipping rules for Northern Ireland. Postcode should start with BT followed by anything else when the country is the UK. The only way possible in the old way was adding all possible exact postcodes. Now it is possible to add a wildcard asterix at the and of a partial postcode. For example BT* will match all postcodes starting with BT like BT1 1AA (a valid postcode from Northern Ireland). The same was already possible in the TAX rates zip/post code.

Related to issue: #1486

Related Pull Requests

#1503

Manual testing scenarios (*)

  1. Add a partial postcode to the table rates. For example BT*.
  2. All orders with postcodes starting with BT should now use this shipping rule.

Questions or comments

The most specific postcode rule that is valid will be applied. So if you have the following postcodes for the same country and state:

  • *
  • BT*
  • BT1*
  • BT1 1AA

And a customer places a order with "BT1 1BB" the line with "BT1*" will be applied. This is the most specific rule that matches because "BT1 1AA" doesn't match and "BT1*" is more specific than "BT*" and "*".

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)

@github-actions github-actions bot added the Component: Shipping Relates to Mage_Shipping label Mar 18, 2021
colinmollenhour
colinmollenhour previously approved these changes Mar 19, 2021
View reviewed changes
Copy link
Member

@colinmollenhour colinmollenhour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting approach, I don't see any issues with it. Thanks for the PR!

@AlterWeb AlterWeb mentioned this pull request Apr 26, 2021
Open
@fballiano fballiano changed the base branch from 1.9.4.x to main May 6, 2023 22:06
@fballiano fballiano dismissed colinmollenhour’s stale review May 6, 2023 22:06

The base branch was changed.

@fballiano
Copy link
Contributor

I've rebased this to main and fixed the conflict, @colinmollenhour or somebody could you recheck/retest it please?

@colinmollenhour
Copy link
Member

Just a visual inspection but looks good.

Minor optimization (I think), but couldn't this:

        $conditions[] = "dest_country_id = :country_id AND dest_region_id = :region_id AND dest_zip = '*'";
        $conditions[] = "dest_country_id = :country_id AND dest_region_id = '0' AND dest_zip = '*'";
        $conditions[] = "dest_country_id = '0' AND dest_region_id = :region_id AND dest_zip = '*'";
        $conditions[] = "dest_country_id = '0' AND dest_region_id = '0' AND dest_zip = '*'";

be replaced with this?

        $conditions[] = "dest_country_id IN(:country_id, 0) AND dest_region_id IN(:region_id, 0) AND dest_zip = '*'";

(in both places where this pattern is used)

Is there anywhere this support for wildcards can be documented? Perhaps on the upload page form field?

@fballiano fballiano changed the title Added partial postcode match to table rate shipping New feature: added partial postcode match to table rate shipping Feb 21, 2024
@fballiano
Copy link
Contributor

I was trying to test this PR but, when the checkout calculates the shipping costs I get:

SQLSTATE[HY093]: Invalid parameter number: number of bound variables does not match number of tokens, query was: SELECT shipping_tablerate.* FROM shipping_tablerate WHERE (website_id = :website_id) AND (condition_name = :condition_name) AND (condition_value <= :condition_value) ORDER BY dest_country_id DESC, dest_region_id DESC, LENGTH(dest_zip) DESC, dest_zip DESC, condition_value DESC LIMIT 1

#0 /Users/fab/Projects/openmage/lib/Varien/Db/Statement/Pdo/Mysql.php(97): Zend_Db_Statement_Pdo->_execute(Array)
#1 /Users/fab/Projects/openmage/vendor/shardj/zf1-future/library/Zend/Db/Statement.php(323): Varien_Db_Statement_Pdo_Mysql->_execute(Array)
#2 /Users/fab/Projects/openmage/vendor/shardj/zf1-future/library/Zend/Db/Adapter/Abstract.php(480): Zend_Db_Statement->execute(Array)
#3 /Users/fab/Projects/openmage/vendor/shardj/zf1-future/library/Zend/Db/Adapter/Pdo/Abstract.php(267): Zend_Db_Adapter_Abstract->query('SELECT `shippin...', Array)
#4 /Users/fab/Projects/openmage/lib/Varien/Db/Adapter/Pdo/Mysql.php(486): Zend_Db_Adapter_Pdo_Abstract->query('SELECT `shippin...', Array)
#5 /Users/fab/Projects/openmage/vendor/shardj/zf1-future/library/Zend/Db/Adapter/Abstract.php(759): Varien_Db_Adapter_Pdo_Mysql->query('SELECT `shippin...', Array)
#6 /Users/fab/Projects/openmage/app/code/core/Mage/Shipping/Model/Resource/Carrier/Tablerate.php(168): Zend_Db_Adapter_Abstract->fetchRow(Object(Varien_Db_Select), Array)
#7 /Users/fab/Projects/openmage/app/code/core/Mage/Shipping/Model/Carrier/Tablerate.php(209): Mage_Shipping_Model_Resource_Carrier_Tablerate->getRate(Object(Mage_Shipping_Model_Rate_Request))
#8 /Users/fab/Projects/openmage/app/code/core/Mage/Shipping/Model/Carrier/Tablerate.php(131): Mage_Shipping_Model_Carrier_Tablerate->getRate(Object(Mage_Shipping_Model_Rate_Request))
#9 /Users/fab/Projects/openmage/app/code/core/Mage/Shipping/Model/Shipping.php(200): Mage_Shipping_Model_Carrier_Tablerate->collectRates(Object(Mage_Shipping_Model_Rate_Request))
#10 /Users/fab/Projects/openmage/app/code/core/Mage/Shipping/Model/Shipping.php(120): Mage_Shipping_Model_Shipping->collectCarrierRates('tablerate', Object(Mage_Shipping_Model_Rate_Request))
#11 /Users/fab/Projects/openmage/app/code/core/Mage/Sales/Model/Quote/Address.php(1026): Mage_Shipping_Model_Shipping->collectRates(Object(Mage_Shipping_Model_Rate_Request))
#12 /Users/fab/Projects/openmage/app/code/core/Mage/Sales/Model/Quote/Address.php(957): Mage_Sales_Model_Quote_Address->requestShippingRates()
#13 /Users/fab/Projects/openmage/app/code/core/Mage/Sales/Model/Quote/Address/Total/Shipping.php(143): Mage_Sales_Model_Quote_Address->collectShippingRates()
#14 /Users/fab/Projects/openmage/app/code/core/Mage/Sales/Model/Quote/Address.php(1095): Mage_Sales_Model_Quote_Address_Total_Shipping->collect(Object(Mage_Sales_Model_Quote_Address))
#15 /Users/fab/Projects/openmage/app/code/core/Mage/Sales/Model/Quote.php(1442): Mage_Sales_Model_Quote_Address->collectTotals()
#16 /Users/fab/Projects/openmage/app/code/core/Mage/Checkout/Model/Cart.php(449): Mage_Sales_Model_Quote->collectTotals()
#17 /Users/fab/Projects/openmage/app/code/core/Mage/Checkout/controllers/CartController.php(151): Mage_Checkout_Model_Cart->save()
#18 /Users/fab/Projects/openmage/app/code/core/Mage/Core/Controller/Varien/Action.php(421): Mage_Checkout_CartController->indexAction()
#19 /Users/fab/Projects/openmage/app/code/core/Mage/Core/Controller/Varien/Router/Standard.php(255): Mage_Core_Controller_Varien_Action->dispatch('index')
#20 /Users/fab/Projects/openmage/app/code/core/Mage/Core/Controller/Varien/Front.php(181): Mage_Core_Controller_Varien_Router_Standard->match(Object(Mage_Core_Controller_Request_Http))
#21 /Users/fab/Projects/openmage/app/code/core/Mage/Core/Model/App.php(358): Mage_Core_Controller_Varien_Front->dispatch()
#22 /Users/fab/Projects/openmage/app/Mage.php(760): Mage_Core_Model_App->run(Array)
#23 /Users/fab/Projects/openmage/index.php(56): Mage::run('', 'store')
#24 {main}

@fballiano
Copy link
Contributor

This was missing from the submitted code:
415d206

and adding it solved #1504 (comment)

fballiano
fballiano approved these changes Feb 21, 2024
View reviewed changes
Copy link
Contributor

@fballiano fballiano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After my changes I tested this PR and it work correctly (tested with *, partial match and full match)

@fballiano fballiano merged commit ef34421 into OpenMage:main Feb 21, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@addison74 addison74 addison74 approved these changes

@empiricompany empiricompany empiricompany approved these changes

@fballiano fballiano fballiano approved these changes

@colinmollenhour colinmollenhour colinmollenhour left review comments

Assignees
No one assigned
Labels
Component: Shipping Relates to Mage_Shipping
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@AlterWeb @fballiano @colinmollenhour @empiricompany @addison74